fix: Windows port conflict + HF_HUB_OFFLINE for offline model loading#85
fix: Windows port conflict + HF_HUB_OFFLINE for offline model loading#85caaaaaleb wants to merge 6 commits into
Conversation
- Implement kill_orphan_on_port for Windows using netstat + taskkill - Set HF_HUB_OFFLINE=1 in main.py before any HF library import - Set HF_HUB_OFFLINE=1 in model_manager._load_model_sync with restore - Add local_files_only=True to OmniVoice.from_pretrained call Fixes "Could not import module AutoModel" error caused by port conflict when stale backend processes occupy the port on Windows. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 Walkthrough✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
|
- 全文翻译为中文 - 添加本 Fork 修复版说明（Windows 端口冲突、离线模式） - 更新下载链接指向本 Fork Release - 增加国内网络环境使用指南 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
README.md (2)
281-293: 💤 Low valueAdd language identifier to fenced code block.
The architecture diagram code block should specify a language identifier to satisfy markdown linting rules. Consider using
textorplaintextfor ASCII art diagrams.📝 Proposed fix
-``` +```text ┌─────────────────────────────────────────────────┐ │ 前端 (React) │As per coding guidelines, static analysis (markdownlint-cli2) reports: "Fenced code blocks should have a language specified (MD040)".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 281 - 293, The fenced ASCII-art diagram block (the triple-backtick block containing the box with "前端 (React)" and "后端 (FastAPI)" lines) needs a language identifier to satisfy MD040; update the opening fence from ``` to ```text (or ```plaintext) so the block becomes a labeled code fence (e.g., ```text) and keep the enclosed ASCII art unchanged.
8-9: 💤 Low valueConsider badge consistency with fork messaging.
The stars badge references the original repository (
debpalash/OmniVoice-Studio) while the release badge references the fork (caaaaaleb/OmniVoice-Studio). This mixing could confuse users about which repository they're viewing.Since this is a documented fork providing fixes, consider either:
- Using fork references for both badges, or
- Adding a note clarifying that stars reflect the upstream project
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 8 - 9, The README mixes upstream and fork badges (stars link uses https://github.com/debpalash/OmniVoice-Studio while release uses https://github.com/caaaaaleb/OmniVoice-Studio); make them consistent by either updating the stars badge URL to the fork (change the href/src that reference debpalash to caaaaaleb) or add a short clarifying sentence beneath the badges stating that the star count reflects the upstream repository; ensure the two anchor/img entries shown in the diff are updated together so both badges point to the same repository or are accompanied by the clarification text.frontend/src-tauri/src/backend.rs (1)
103-109: ⚡ Quick winSurface cleanup failures in the logs.
This branch silently returns when
netstatcannot be launched and ignorestaskkillexit status. If cleanup fails because of permissions or a missing binary, we lose the only clue and fall back to the same bind error this PR is trying to fix.Proposed change
let out = match std::process::Command::new("netstat") .args(["-ano", "-p", "TCP"]) .output() { Ok(o) => o, - Err(_) => return, + Err(e) => { + log::warn!("Failed to run netstat for port {}: {}", port, e); + return; + } }; + if !out.status.success() { + log::warn!( + "netstat exited with status {:?} while probing port {}: {}", + out.status.code(), + port, + String::from_utf8_lossy(&out.stderr) + ); + return; + } ... - let _ = std::process::Command::new("taskkill") - .args(["/PID", &pid.to_string(), "/F"]) - .output(); + match std::process::Command::new("taskkill") + .args(["/PID", &pid.to_string(), "/F"]) + .output() + { + Ok(kill) if !kill.status.success() => { + log::warn!( + "taskkill failed for pid {} on port {}: {}", + pid, + port, + String::from_utf8_lossy(&kill.stderr) + ); + } + Err(e) => { + log::warn!( + "Failed to invoke taskkill for pid {} on port {}: {}", + pid, + port, + e + ); + } + _ => {} + }Also applies to: 135-137
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src-tauri/src/backend.rs` around lines 103 - 109, The code silently returns when std::process::Command::new("netstat").output() fails and also ignores the exit status of the subsequent taskkill invocation; update the Err(_) arm of the Command::new("netstat")...output() match to log the actual error (include the error from output()) instead of returning, and after spawning/running taskkill check its ExitStatus and log a clear error if it failed (include stdout/stderr if available) so permission/missing-binary failures are visible; update the same pattern referenced around the other invocation (lines near 135-137) to perform the same logging and non-silent failure handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src-tauri/src/backend.rs`:
- Around line 128-137: The current Windows branch blindly taskkills whatever PID
netstat reports; update the logic around the PID extraction (the block that
parses parts/ pid_str and produces pid and the taskkill call) to first query the
target process command line or executable and verify it belongs to our spawned
OmniVoice backend before killing: after parsing pid, run a safe metadata lookup
(e.g. via PowerShell/Get-Process or wmic for the given pid) to obtain
ProcessName/Path/CommandLine, compare that value against the expected backend
executable name or unique marker used when we spawn the backend, and only call
std::process::Command::new("taskkill") if the verification matches; otherwise
log a port-conflict error and surface the conflict instead of killing the
process.
---
Nitpick comments:
In `@frontend/src-tauri/src/backend.rs`:
- Around line 103-109: The code silently returns when
std::process::Command::new("netstat").output() fails and also ignores the exit
status of the subsequent taskkill invocation; update the Err(_) arm of the
Command::new("netstat")...output() match to log the actual error (include the
error from output()) instead of returning, and after spawning/running taskkill
check its ExitStatus and log a clear error if it failed (include stdout/stderr
if available) so permission/missing-binary failures are visible; update the same
pattern referenced around the other invocation (lines near 135-137) to perform
the same logging and non-silent failure handling.
In `@README.md`:
- Around line 281-293: The fenced ASCII-art diagram block (the triple-backtick
block containing the box with "前端 (React)" and "后端 (FastAPI)" lines) needs a
language identifier to satisfy MD040; update the opening fence from ``` to
```text (or ```plaintext) so the block becomes a labeled code fence (e.g.,
```text) and keep the enclosed ASCII art unchanged.
- Around line 8-9: The README mixes upstream and fork badges (stars link uses
https://github.com/debpalash/OmniVoice-Studio while release uses
https://github.com/caaaaaleb/OmniVoice-Studio); make them consistent by either
updating the stars badge URL to the fork (change the href/src that reference
debpalash to caaaaaleb) or add a short clarifying sentence beneath the badges
stating that the star count reflects the upstream repository; ensure the two
anchor/img entries shown in the diff are updated together so both badges point
to the same repository or are accompanied by the clarification text.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d3c53b90-30d8-4ff7-9193-1a53c28c786e
📒 Files selected for processing (4)
README.mdbackend/main.pybackend/services/model_manager.pyfrontend/src-tauri/src/backend.rs
| let parts: Vec<&str> = line.split_whitespace().collect(); | ||
| if let Some(pid_str) = parts.last() { | ||
| if let Ok(pid) = pid_str.parse::<u32>() { | ||
| log::warn!( | ||
| "Killing orphan process {} on port {} (Windows)", | ||
| pid, port | ||
| ); | ||
| let _ = std::process::Command::new("taskkill") | ||
| .args(["/PID", &pid.to_string(), "/F"]) | ||
| .output(); |
There was a problem hiding this comment.
Don't taskkill arbitrary listeners on this port.
Line 135 force-kills whichever PID netstat reports, but this helper never verifies that the process is actually our backend. If some other local service is bound to the configured port, startup can terminate an unrelated app instead of surfacing a port conflict. Please check the process command line/executable matches the spawned OmniVoice backend before killing it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src-tauri/src/backend.rs` around lines 128 - 137, The current
Windows branch blindly taskkills whatever PID netstat reports; update the logic
around the PID extraction (the block that parses parts/ pid_str and produces pid
and the taskkill call) to first query the target process command line or
executable and verify it belongs to our spawned OmniVoice backend before
killing: after parsing pid, run a safe metadata lookup (e.g. via
PowerShell/Get-Process or wmic for the given pid) to obtain
ProcessName/Path/CommandLine, compare that value against the expected backend
executable name or unique marker used when we spawn the backend, and only call
std::process::Command::new("taskkill") if the verification matches; otherwise
log a port-conflict error and surface the conflict instead of killing the
process.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 11-13: The directory tree lists both README.md and 使用指南.md but the
tree labels 使用指南.md as “本文件” while this PR edits README.md, causing confusion;
update the directory tree entry so the “本文件” label correctly refers to README.md
(or rename the file consistently) by changing the label for 使用指南.md to its
proper description and/or marking README.md as “本文件”, and ensure the README.md
and 使用指南.md filenames and their descriptive comments (in the README listing) are
consistent.
- Around line 9-25: The README.md's directory tree fenced code block is missing
a language tag (causing MD040); open the fenced block in README.md and add a
language tag (e.g., replace "```" with "```text") so the tree block is
recognized as plain text; locate the block shown in the diff (the directory tree
starting with "E:\OmniVoice-Studio\") and update its opening fence accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| ``` | ||
|
|
||
| Open normally after. One-time fix. | ||
| </details> | ||
|
|
||
| <details> | ||
| <summary><b>Windows — first launch takes 5–10 minutes</b></summary> | ||
| <br/> | ||
|
|
||
| The app bootstraps a Python virtual environment, installs dependencies, and downloads ffmpeg on first run. The splash screen shows each step. Subsequent launches start in seconds. | ||
| </details> | ||
|
|
||
| <details> | ||
| <summary><b>Linux — AppImage needs FUSE</b></summary> | ||
| <br/> | ||
|
|
||
| If FUSE isn't available, use the `.deb` package or extract-and-run: | ||
|
|
||
| ```bash | ||
| chmod +x OmniVoice.Studio_*.AppImage | ||
| ./OmniVoice.Studio_*.AppImage --appimage-extract-and-run | ||
| ``` | ||
| </details> | ||
|
|
||
| <details> | ||
| <summary><b>Linux — White screen on Fedora 44 / Ubuntu 24.04</b></summary> | ||
| <br/> | ||
|
|
||
| Some newer distros ship a WebKit/GTK version with compositing issues. Try: | ||
|
|
||
| ```bash | ||
| WEBKIT_DISABLE_COMPOSITING_MODE=1 ./OmniVoice.Studio_*.AppImage | ||
| E:\OmniVoice-Studio\ | ||
| ├── README.md # 项目说明（中文） | ||
| ├── 使用指南.md # 本文件 | ||
| ├── OmniVoice_Studio_0.2.7_x64.msi # Windows 安装包 (~7 MB) | ||
| ├── backend/ # Python 后端 | ||
| │ ├── main.py # FastAPI 入口 | ||
| │ └── services/ | ||
| │ └── model_manager.py # 模型加载 | ||
| ├── frontend/ # React 前端 (Tauri) | ||
| │ └── src-tauri/ | ||
| │ └── src/ | ||
| │ └── backend.rs # 后端进程管理 | ||
| ├── omnivoice/ # OmniVoice Python 包 | ||
| ├── pyproject.toml # Python 项目配置 | ||
| └── uv.lock # 依赖锁定文件 | ||
| ``` |
There was a problem hiding this comment.
Add a language tag to the fenced code block to satisfy markdownlint.
Line 9 opens a fenced block without a language, triggering MD040 and causing lint noise.
Suggested patch
-```
+```text
E:\OmniVoice-Studio\
├── README.md # 项目说明（中文）
...
└── uv.lock # 依赖锁定文件</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>
[warning] 9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @README.md around lines 9 - 25, The README.md's directory tree fenced code
block is missing a language tag (causing MD040); open the fenced block in
README.md and add a language tag (e.g., replace "" with "text") so the
tree block is recognized as plain text; locate the block shown in the diff (the
directory tree starting with "E:\OmniVoice-Studio") and update its opening
fence accordingly.
</details>
<!-- fingerprinting:phantom:triton:hawk -->
<!-- This is an auto-generated comment by CodeRabbit -->
- Add complete zh/en i18n across 50+ source files with i18next - Add language switcher (中/EN toggle) in header with localStorage persistence - Fix _gpu_pool NameError in model_manager.py (__getattr__ internal access) - Fix TritonMissing on Windows: disable torch.compile/dynamo/inductor via env vars + explicit check - Fix Vite IPv6-only binding: force host to 127.0.0.1 - Add start.bat (production, .venv only) and start-dev.bat (Vite HMR) - Rebuild frontend dist with all fixes - Update README with new features and fix table Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
start.bat (1)
1-66:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical: Convert file to Windows line endings (CRLF).
The file uses Unix line endings (LF-only), which will cause parsing failures on Windows due to CMD.exe parser bugs. Batch files require CRLF line endings to work reliably.
🔧 How to fix
Convert the file to Windows line endings using one of:
Git:
# Configure git to auto-convert on checkout (run once in repo root) git config core.autocrlf true git rm --cached start.bat git reset --hardCommand line:
# Using unix2dos (if available) unix2dos start.bat # Or using PowerShell (Get-Content start.bat) | Set-Content start.batEditor:
- VS Code: Click "LF" in status bar → select "CRLF"
- Notepad++: Edit → EOL Conversion → Windows (CR LF)
As per coding guidelines, every fix must work on Windows (x64) with no platform-only regressions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@start.bat` around lines 1 - 66, The start.bat file currently uses LF-only line endings which will break CMD parsing; convert start.bat to Windows CRLF endings (ensure every line ends with CRLF) and commit that change; you can do this by enabling Git autocrlf and re-checking out the file (git config core.autocrlf true; git rm --cached start.bat; git reset --hard), or run a conversion tool (unix2dos start.bat) or change the EOL in your editor (e.g., VS Code: click LF → select CRLF), then verify the batch commands like the for loops, start "OmniVoice-Backend", curl health check and taskkill lines run correctly on Windows before committing.start-dev.bat (1)
1-89:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical: Convert file to Windows line endings (CRLF).
The file uses Unix line endings (LF-only), which will cause parsing failures on Windows due to CMD.exe parser bugs. Batch files require CRLF line endings to work reliably.
🔧 How to fix
Convert the file to Windows line endings using one of:
Git:
# Configure git to auto-convert on checkout (run once in repo root) git config core.autocrlf true git rm --cached start-dev.bat git reset --hardCommand line:
# Using unix2dos (if available) unix2dos start-dev.bat # Or using PowerShell (Get-Content start-dev.bat) | Set-Content start-dev.batEditor:
- VS Code: Click "LF" in status bar → select "CRLF"
- Notepad++: Edit → EOL Conversion → Windows (CR LF)
As per coding guidelines, every fix must work on Windows (x64) with no platform-only regressions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@start-dev.bat` around lines 1 - 89, The batch script uses LF-only line endings which break CMD parsing; convert start-dev.bat to Windows CRLF line endings and commit the change so Windows can execute it reliably. Fix by converting the file (e.g., run unix2dos on start-dev.bat, use PowerShell Set-Content to rewrite with CRLF, enable git core.autocrlf and refresh the file, or change EOL to CRLF in your editor/VS Code) and verify the top of the script (starts with "`@echo` off") runs successfully on a Windows x64 environment before pushing.frontend/src/pages/Projects.jsx (1)
163-176:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winVariable shadowing causes runtime error.
The loop variable
tat line 163 shadows the translation functiontfromuseTranslation()declared at line 90. When line 167 attempts to callt('projects.transcription_item'), it will fail withTypeError: t is not a functionbecausetnow refers to the current transcription object, not the translation function.🐛 Proposed fix: rename loop variable
- for (const t of transcriptions) { + for (const trans of transcriptions) { list.push({ type: 'transcripts', - id: t.id || String(Math.random()), - title: (t.text || t('projects.transcription_item')).slice(0, 120), - subtitle: [t.language, t.duration_s ? `${Math.round(t.duration_s)}s` : ''].filter(Boolean).join(' · '), - ts: t.timestamp ? Date.parse(t.timestamp) : 0, + id: trans.id || String(Math.random()), + title: (trans.text || t('projects.transcription_item')).slice(0, 120), + subtitle: [trans.language, trans.duration_s ? `${Math.round(trans.duration_s)}s` : ''].filter(Boolean).join(' · '), + ts: trans.timestamp ? Date.parse(trans.timestamp) : 0, accent: '`#83a598`', Icon: FileText, onClick: () => { - navigator.clipboard.writeText(t.text || ''); + navigator.clipboard.writeText(trans.text || ''); }, }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/pages/Projects.jsx` around lines 163 - 176, The loop is shadowing the translation function t from useTranslation() by using `for (const t of transcriptions)`, causing `t('projects.transcription_item')` to fail; rename the loop variable (e.g., `for (const transcript of transcriptions)` or `tr`) and update all references inside the loop (`t.id`, `t.text`, `t.language`, `t.duration_s`, `t.timestamp`) to the new name so the translation function `t` remains callable.
🧹 Nitpick comments (2)
frontend/src/components/ExportModal.jsx (1)
76-80: 💤 Low valueConsider adding language to useMemo dependencies.
The
useMemousest('dub.original')but only depends ondubTracks. While the component will re-render when language changes (via react-i18next context), explicitly tracking language in dependencies makes the intent clearer and satisfies ESLint exhaustive-deps.📦 Suggested addition to dependencies
+import { useTranslation } from 'react-i18next'; + export default function ExportModal({ ... }) { const { t, i18n } = useTranslation(); ... const allTracks = useMemo(() => { const out = [{ code: 'original', label: t('dub.original'), kind: 'original' }]; (dubTracks || []).forEach(t => out.push({ code: t, label: t.toUpperCase(), kind: 'dub' })); return out; - }, [dubTracks]); + }, [dubTracks, i18n.language, t]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/ExportModal.jsx` around lines 76 - 80, The memo for allTracks uses the i18n translator t('dub.original') but only lists dubTracks in its dependency array; update the useMemo dependencies for the allTracks constant (the useMemo call that builds out from dubTracks) to include the translator/language so the memo updates when language changes—e.g., add either the t function or the current i18n.language value to the dependency array to satisfy exhaustive-deps and ensure labels update on locale change.frontend/src/components/GlossaryPanel.jsx (1)
175-182: 💤 Low valueConsider renaming map parameter to avoid shadowing.
The loop variable
tshadows the translation function fromuseTranslation(). While this doesn't cause a runtime error (since the translation function isn't called inside the map), it reduces code clarity and could confuse maintainers.♻️ Suggested refactor
- {terms.map(t => ( + {terms.map(term => ( <GlossaryRow - key={t.id} - term={t} - onUpdate={(patch) => onUpdate(t.id, patch)} - onDelete={() => onDelete(t.id)} + key={term.id} + term={term} + onUpdate={(patch) => onUpdate(term.id, patch)} + onDelete={() => onDelete(term.id)} /> ))}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/GlossaryPanel.jsx` around lines 175 - 182, Rename the map loop parameter that currently uses "t" to avoid shadowing the translation function from useTranslation(); for example, change the iterator in the terms.map callback to "term" or "termItem" and update all references inside the callback (key, term prop, onUpdate/onDelete closures) so GlossaryRow receives the same data but no longer conflicts with the translation function name.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/App.jsx`:
- Around line 216-218: The default compareText initializer references an
undefined i18n instead of using the translation hook; inside the App component
call useTranslation() (e.g., const { t } = useTranslation()) and update the
useState initializer for compareText to use t('voice.default_test_phrase') (with
a try/catch or fallback string) so the localized default is used; update any
usages that referenced i18n.t to use the destructured t function and ensure
useTranslation is imported and invoked within the App component scope.
In `@frontend/src/pages/CloneDesignTab.jsx`:
- Line 211: The concatenation at the JSX expression using
t('voice.using_profile') + profiles.find(p => p.id === selectedProfile)?.name
breaks localization; instead pass the profile name as an interpolation variable
to the translator. Replace the concatenated expression with a single t call that
supplies the name (e.g., t('voice.using_profile', { name: profiles.find(p =>
p.id === selectedProfile)?.name })) so translators control spacing/word order;
update the JSX where selectedProfile and profiles are referenced accordingly.
In `@frontend/src/pages/Transcriptions.jsx`:
- Around line 76-82: The useCallback hooks for copyText and exportAll capture
the translation function t but do not include it in their dependency arrays,
causing stale translations after language changes; update the useCallback
declarations for copyText and exportAll to include t (e.g., useCallback(..., [t,
...otherDeps])) so they re-create with the latest translator when language
switches.
---
Outside diff comments:
In `@frontend/src/pages/Projects.jsx`:
- Around line 163-176: The loop is shadowing the translation function t from
useTranslation() by using `for (const t of transcriptions)`, causing
`t('projects.transcription_item')` to fail; rename the loop variable (e.g., `for
(const transcript of transcriptions)` or `tr`) and update all references inside
the loop (`t.id`, `t.text`, `t.language`, `t.duration_s`, `t.timestamp`) to the
new name so the translation function `t` remains callable.
In `@start-dev.bat`:
- Around line 1-89: The batch script uses LF-only line endings which break CMD
parsing; convert start-dev.bat to Windows CRLF line endings and commit the
change so Windows can execute it reliably. Fix by converting the file (e.g., run
unix2dos on start-dev.bat, use PowerShell Set-Content to rewrite with CRLF,
enable git core.autocrlf and refresh the file, or change EOL to CRLF in your
editor/VS Code) and verify the top of the script (starts with "`@echo` off") runs
successfully on a Windows x64 environment before pushing.
In `@start.bat`:
- Around line 1-66: The start.bat file currently uses LF-only line endings which
will break CMD parsing; convert start.bat to Windows CRLF endings (ensure every
line ends with CRLF) and commit that change; you can do this by enabling Git
autocrlf and re-checking out the file (git config core.autocrlf true; git rm
--cached start.bat; git reset --hard), or run a conversion tool (unix2dos
start.bat) or change the EOL in your editor (e.g., VS Code: click LF → select
CRLF), then verify the batch commands like the for loops, start
"OmniVoice-Backend", curl health check and taskkill lines run correctly on
Windows before committing.
---
Nitpick comments:
In `@frontend/src/components/ExportModal.jsx`:
- Around line 76-80: The memo for allTracks uses the i18n translator
t('dub.original') but only lists dubTracks in its dependency array; update the
useMemo dependencies for the allTracks constant (the useMemo call that builds
out from dubTracks) to include the translator/language so the memo updates when
language changes—e.g., add either the t function or the current i18n.language
value to the dependency array to satisfy exhaustive-deps and ensure labels
update on locale change.
In `@frontend/src/components/GlossaryPanel.jsx`:
- Around line 175-182: Rename the map loop parameter that currently uses "t" to
avoid shadowing the translation function from useTranslation(); for example,
change the iterator in the terms.map callback to "term" or "termItem" and update
all references inside the callback (key, term prop, onUpdate/onDelete closures)
so GlossaryRow receives the same data but no longer conflicts with the
translation function name.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a747c065-c8fb-4fe7-8464-b682031eb714
📒 Files selected for processing (56)
.gitignoreREADME.mdbackend/main.pybackend/services/model_manager.pyfrontend/src/App.jsxfrontend/src/components/AudioTrimmer.jsxfrontend/src/components/BatchAddDialog.jsxfrontend/src/components/BootstrapSplash.jsxfrontend/src/components/CaptureWidget.jsxfrontend/src/components/CastingView.jsxfrontend/src/components/CheckpointBanner.jsxfrontend/src/components/CompareModal.jsxfrontend/src/components/DirectionDialog.jsxfrontend/src/components/DubSegmentRow.jsxfrontend/src/components/DubSegmentTable.jsxfrontend/src/components/ErrorBoundary.jsxfrontend/src/components/ExportModal.jsxfrontend/src/components/FloatingPill.jsxfrontend/src/components/GlossaryPanel.jsxfrontend/src/components/Header.jsxfrontend/src/components/KeyboardCheatsheet.jsxfrontend/src/components/LogsFooter.jsxfrontend/src/components/MultiLangPicker.jsxfrontend/src/components/NavRail.jsxfrontend/src/components/NotificationPanel.jsxfrontend/src/components/ReadinessChecklist.jsxfrontend/src/components/SearchableSelect.jsxfrontend/src/components/Sidebar.jsxfrontend/src/components/StoriesEditor.jsxfrontend/src/components/VoicePreview.jsxfrontend/src/components/WaveformTimeline.jsxfrontend/src/hooks/useAppData.jsfrontend/src/hooks/useDubWorkflow.jsfrontend/src/hooks/useProfiles.jsfrontend/src/hooks/useTTS.jsfrontend/src/i18n/index.tsfrontend/src/i18n/locales/en.jsonfrontend/src/i18n/locales/zh.jsonfrontend/src/main-app.jsxfrontend/src/pages/BatchQueue.jsxfrontend/src/pages/CloneDesignTab.jsxfrontend/src/pages/DonatePage.jsxfrontend/src/pages/DubTab.jsxfrontend/src/pages/EnterprisePage.jsxfrontend/src/pages/Launchpad.jsxfrontend/src/pages/Projects.jsxfrontend/src/pages/Settings.jsxfrontend/src/pages/SetupWizard.jsxfrontend/src/pages/ToolsPage.jsxfrontend/src/pages/Transcriptions.jsxfrontend/src/pages/VoiceGallery.jsxfrontend/src/pages/VoiceProfile.jsxfrontend/src/ui/Dialog.jsxfrontend/vite.config.jsstart-dev.batstart.bat
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/main.py
| const [compareText, setCompareText] = useState(() => { | ||
| try { return i18n?.t?.('voice.default_test_phrase'); } catch { return "The quick brown fox jumps over the lazy dog."; } | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP '\bi18n\?\.t\?\(' frontend/src/App.jsx
rg -nP '^import .*\\bi18n\\b' frontend/src/App.jsxRepository: debpalash/OmniVoice-Studio
Length of output: 52
🏁 Script executed:
#!/bin/bash
# Check if file exists and get line count
ls -la frontend/src/App.jsx 2>/dev/null || echo "File not found at frontend/src/App.jsx"
wc -l frontend/src/App.jsx 2>/dev/null || echo "Cannot count lines"Repository: debpalash/OmniVoice-Studio
Length of output: 167
🏁 Script executed:
#!/bin/bash
# Read lines 216-218 and surrounding context
if [ -f frontend/src/App.jsx ]; then
echo "=== Lines 210-225 ==="
sed -n '210,225p' frontend/src/App.jsx | cat -n
fiRepository: debpalash/OmniVoice-Studio
Length of output: 985
🏁 Script executed:
#!/bin/bash
# Search for import statements related to i18n, translation, or useTranslation
if [ -f frontend/src/App.jsx ]; then
echo "=== Import statements for i18n/translation ==="
head -50 frontend/src/App.jsx | grep -nE 'import.*i18n|import.*useTranslation|import.*translation'
fiRepository: debpalash/OmniVoice-Studio
Length of output: 167
🏁 Script executed:
#!/bin/bash
# Check for all imports at the top of the file
if [ -f frontend/src/App.jsx ]; then
echo "=== First 60 lines (imports section) ==="
head -60 frontend/src/App.jsx | cat -n
fiRepository: debpalash/OmniVoice-Studio
Length of output: 3581
🏁 Script executed:
#!/bin/bash
# Search for useTranslation usage in App.jsx
rg -n 'useTranslation\|const \{ t \}' frontend/src/App.jsx | head -20Repository: debpalash/OmniVoice-Studio
Length of output: 52
Undefined i18n reference prevents localized default compare text.
Line 217 references i18n without a binding. The useTranslation hook is imported but never instantiated in the App component, so the i18n object doesn't exist and the try/catch always falls back to English text.
Call useTranslation() to destructure the t function and use it directly:
Suggested fix
+ const { t } = useTranslation();
+
const [compareText, setCompareText] = useState(() => {
- try { return i18n?.t?.('voice.default_test_phrase'); } catch { return "The quick brown fox jumps over the lazy dog."; }
+ return t('voice.default_test_phrase', { defaultValue: 'The quick brown fox jumps over the lazy dog.' });
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [compareText, setCompareText] = useState(() => { | |
| try { return i18n?.t?.('voice.default_test_phrase'); } catch { return "The quick brown fox jumps over the lazy dog."; } | |
| }); | |
| const { t } = useTranslation(); | |
| const [compareText, setCompareText] = useState(() => { | |
| return t('voice.default_test_phrase', { defaultValue: 'The quick brown fox jumps over the lazy dog.' }); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/App.jsx` around lines 216 - 218, The default compareText
initializer references an undefined i18n instead of using the translation hook;
inside the App component call useTranslation() (e.g., const { t } =
useTranslation()) and update the useState initializer for compareText to use
t('voice.default_test_phrase') (with a try/catch or fallback string) so the
localized default is used; update any usages that referenced i18n.t to use the
destructured t function and ensure useTranslation is imported and invoked within
the App component scope.
| <div className="clone-profile-banner"> | ||
| <span className="clone-profile-banner__label"> | ||
| Using profile: {profiles.find(p => p.id === selectedProfile)?.name} | ||
| {t('voice.using_profile')}{profiles.find(p => p.id === selectedProfile)?.name} |
There was a problem hiding this comment.
Use interpolation for the selected-profile label.
Line 211 concatenates translated text with the profile name, which breaks spacing/word order in some locales. Pass name through t(...) interpolation instead.
Proposed fix
- {t('voice.using_profile')}{profiles.find(p => p.id === selectedProfile)?.name}
+ {t('voice.using_profile', {
+ name: profiles.find(p => p.id === selectedProfile)?.name ?? '',
+ })}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {t('voice.using_profile')}{profiles.find(p => p.id === selectedProfile)?.name} | |
| {t('voice.using_profile', { | |
| name: profiles.find(p => p.id === selectedProfile)?.name ?? '', | |
| })} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/pages/CloneDesignTab.jsx` at line 211, The concatenation at the
JSX expression using t('voice.using_profile') + profiles.find(p => p.id ===
selectedProfile)?.name breaks localization; instead pass the profile name as an
interpolation variable to the translator. Replace the concatenated expression
with a single t call that supplies the name (e.g., t('voice.using_profile', {
name: profiles.find(p => p.id === selectedProfile)?.name })) so translators
control spacing/word order; update the JSX where selectedProfile and profiles
are referenced accordingly.
| const copyText = useCallback((text) => { | ||
| navigator.clipboard.writeText(text).then( | ||
| () => toast.success('Copied to clipboard'), | ||
| () => toast.error('Copy failed') | ||
| () => toast.success(t('transcriptions.copied_to_clipboard')), | ||
| () => toast.error(t('transcriptions.copy_failed')) | ||
| ); | ||
| }, []); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify callback dependency arrays around translated toasts.
rg -n -C2 "const copyText = useCallback|const exportAll = useCallback|toast\\.success\\(|toast\\.error\\(" frontend/src/pages/Transcriptions.jsxRepository: debpalash/OmniVoice-Studio
Length of output: 681
Add t to callback dependency arrays to prevent stale translations after language switch.
The copyText (line 76) and exportAll (line 96) callbacks use the t function but omit it from their dependency arrays. This causes the callbacks to retain old translations when the language changes at runtime.
🔧 Proposed fix
const copyText = useCallback((text) => {
navigator.clipboard.writeText(text).then(
() => toast.success(t('transcriptions.copied_to_clipboard')),
() => toast.error(t('transcriptions.copy_failed'))
);
- }, []);
+ }, [t]);
@@
const exportAll = useCallback(() => {
@@
URL.revokeObjectURL(url);
toast.success(t('transcriptions.exported_transcriptions'));
- }, [transcriptions]);
+ }, [transcriptions, t]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/pages/Transcriptions.jsx` around lines 76 - 82, The useCallback
hooks for copyText and exportAll capture the translation function t but do not
include it in their dependency arrays, causing stale translations after language
changes; update the useCallback declarations for copyText and exportAll to
include t (e.g., useCallback(..., [t, ...otherDeps])) so they re-create with the
latest translator when language switches.
|
Thanks @caaaaaleb — the Windows However the PR also rewrites the README into Simplified Chinese, flips the i18n Could you split this into:
One small nit on (1): in the Windows Coordination: The Happy to walk through the split if helpful. Looking forward to landing the backend fixes — they're solid work. |
feat: voice design personality presets with gender/age/pitch/accent attrs - Add structured attrs (Gender, Age, Pitch, Style, EnglishAccent, ChineseDialect) to all 6 personality presets in personalities.py, giving each a distinct voice profile instead of relying solely on the free-text instruct field. - CloneDesignTab: apply personality attrs via setVdStates when selecting a preset, replacing the bare setInstruct call. - useTTS: remove the redundant instruct.trim() append to the prompt parts — attrs now drive the voice-design parameters, and including the raw instruct string was double-counting what attrs already express. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> @
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/pages/CloneDesignTab.jsx (1)
61-68:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestore instruct application when applying a personality.
At Line 67, only
p.attrsis applied.p.instructis no longer applied, so personality presets are only partially reflected in the UI state.Proposed fix
const applyPersonality = (p) => { if (activePersonality === p.id) { setActivePersonality(''); return; } setActivePersonality(p.id); - if (p.attrs) setVdStates(p.attrs); + if (p.attrs) setVdStates(prev => ({ ...prev, ...p.attrs })); + if (p.instruct) setInstruct(p.instruct); };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/pages/CloneDesignTab.jsx` around lines 61 - 68, applyPersonality currently only applies p.attrs via setVdStates and therefore doesn't restore the personality's instruct text; update applyPersonality to also apply p.instruct when present (e.g., call the component state updater that holds the instruct text, such as setInstruct(p.instruct)) when activating a personality, while leaving the existing behavior for deactivation (setActivePersonality('')) intact; ensure you check p.instruct is defined before setting it so you don't overwrite existing instruct state with undefined.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@frontend/src/pages/CloneDesignTab.jsx`:
- Around line 61-68: applyPersonality currently only applies p.attrs via
setVdStates and therefore doesn't restore the personality's instruct text;
update applyPersonality to also apply p.instruct when present (e.g., call the
component state updater that holds the instruct text, such as
setInstruct(p.instruct)) when activating a personality, while leaving the
existing behavior for deactivation (setActivePersonality('')) intact; ensure you
check p.instruct is defined before setting it so you don't overwrite existing
instruct state with undefined.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1594e2e8-07d8-4cb8-a181-83347846c43d
📒 Files selected for processing (3)
backend/core/personalities.pyfrontend/src/hooks/useTTS.jsfrontend/src/pages/CloneDesignTab.jsx
💤 Files with no reviewable changes (1)
- frontend/src/hooks/useTTS.js
|
Triage (v0.3.0 line): this PR is currently conflicting with
Could you rebase onto current |
|
The three fixes here are genuinely valuable — the Windows The problem is scope: this PR touches 58 files / +4006 / −1861, but ~50 of those are whole-frontend reformatting churn (App.jsx, AudioTrimmer, every component) unrelated to the fixes. That looks like fork drift / an editor reformat. We can't review or land that safely — it'd conflict heavily with Could you scope it down to just the fix files? Based on the diff that's roughly:
A clean branch off current |
|
Thanks so much for this, @caaaaaleb — the Windows fixes here are genuinely valuable and we want them in. 🙏 We've salvaged the two safe, high-value pieces into a clean PR (#156), with you credited as co-author on the commit:
A few things we intentionally didn't carry over, and why:
We're leaving this PR open for you / a maintainer to close at your discretion. Thank you again for the contribution — the port-conflict and Triton fixes are landing because of you. |
…aged from #85) (#156) * fix(windows): port-conflict kill + Triton/torch.compile disable (salvaged from #85) Salvages the two safe, valuable Windows fixes from community PR #85 without the changes that would regress all users. backend.rs — implement the Windows branch of `kill_orphan_on_port`, which was a no-op (`pub fn kill_orphan_on_port(_port: u16) {}`). It now parses `netstat -ano -p TCP` for the LISTENING socket on exactly `port` (suffix-matched on ":PORT" to avoid e.g. :3900 matching 39000) and kills the owning PID via `taskkill /PID <pid> /F`. Behind `#[cfg(not(unix))]`; the unix branch is untouched. Signature now matches the unix branch. main.py — on Windows (sys.platform == "win32"), default TORCH_COMPILE_DISABLE / TORCHDYNAMO_DISABLE / TORCHINDUCTOR_DISABLE to "1" before torch is imported. Triton has no Windows wheel, so these prevent TritonMissing/dynamo errors. Uses os.environ.setdefault (never overrides an explicit user value) and is win32-guarded, so cross-platform default behavior is unchanged. Intentionally NOT salvaged from #85: - The unconditional `os.environ["HF_HUB_OFFLINE"] = "1"` in main.py and the `local_files_only=True` / HF_HUB_OFFLINE save-restore in model_manager.py. This breaks first-run model downloads for every user (downloading models on first use is the core value prop). Offline mode must stay opt-in — only when the user sets HF_HUB_OFFLINE themselves. - The model_manager.py torch.compile/_get_gpu_pool changes: already present on main in a superior form (`should_torch_compile()` gating from plan-02/#65 and the existing `_get_gpu_pool()` lazy pool), so applying #85's cruder `TORCH_COMPILE_DISABLE` env check would regress. - The 512-line README rewrite, the ~50 frontend .jsx formatting-only diffs, and the personalities.py attrs additions (out of scope). Refs #85. Co-Authored-By: caaaaaleb <caaaaaleb@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: move win32 torch-disable block below sys.path preamble test_main_py_bootstrap_adds_backend_dir asserts the first 15 lines of main.py contain sys.path.insert + _backend_dir. The win32 block was placed above the preamble, pushing them out of range. The torch env vars only need to precede torch's (lazy) import, so moving the block below the sys.path bootstrap keeps behavior identical and restores the test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: caaaaaleb <caaaaaleb@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Your Windows fixes shipped via #156 (merged) — the |
2 participants
Summary
Three fixes to improve reliability on Windows and restricted networks:
1. Windows
kill_orphan_on_portimplementationThe Windows version was an empty function, causing stale backend processes to occupy the port. When a new backend spawned, uvicorn couldn't bind → shutdown → model loading continued in half-dead process → confusing "Could not import module AutoModel" error.
Fix: Implement
kill_orphan_on_portfor Windows usingnetstat -anoto find the listening PID andtaskkill /PID /Fto terminate it.2.
HF_HUB_OFFLINE=1at import timeOn restricted networks, lazy imports deep in the HuggingFace stack could trigger
httpx.ConnectTimeoutbeforelocal_files_only=Truetakes effect.Fix: Set
HF_HUB_OFFLINE=1inmain.pybefore ANY library import, and inmodel_manager._load_model_sync()with proper save/restore.3.
local_files_only=Trueforfrom_pretrainedAdded
local_files_only=TruetoOmniVoice.from_pretrained()call as belt-and-suspenders alongsideHF_HUB_OFFLINE.Tested
/system/inforeturns correct model checkpoint infoSummary by CodeRabbit
Documentation
New Features
Bug Fixes
Chores